Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pool Overview redesign #2435

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Pool Overview redesign #2435

wants to merge 22 commits into from

Conversation

kattylucy
Copy link
Contributor

@kattylucy kattylucy commented Sep 6, 2024

Copy link

github-actions bot commented Sep 6, 2024

PR deployed in Google Cloud
URL: https://pr2435-app-ff-production.k-f.dev
Commit #: eac775f
To access the functions directly check the corresponding deploy Action

Copy link

github-actions bot commented Sep 6, 2024

PR deployed in Google Cloud
URL: https://app-pr2435.k-f.dev
Commit #: eac775f
To access the functions directly check the corresponding deploy Action

Base automatically changed from add_tile_design to main September 10, 2024 14:13
@kattylucy kattylucy changed the title Style new header Pool Overview redesign Sep 10, 2024
@kattylucy kattylucy force-pushed the pool_overview_redesign branch 11 times, most recently from 1013fe1 to a392405 Compare September 16, 2024 19:41
const data: ChartData[] = React.useMemo(
() =>
truncatedPoolStates?.map((day) => {
truncatedPoolStates?.map((day: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
truncatedPoolStates?.map((day: any) => {
truncatedPoolStates?.map((day) => {

better to make sure truncatedPoolStates is typed rather than casting day as any

<Box width="8px" height="8px" borderRadius="50%" backgroundColor={color} marginRight="4px" />
)

const navObj = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const navObj = {
const navData = {

for consistency though out


const graphData = selectedTabIndex === 0 ? tokenData : apyData

const toggleRange = (e: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's try and avoid anys please!

</Shelf>
<Box display="flex" justifyContent="space-between" alignItems="center">
<Box display="flex" justifyContent="space-evenly">
{graphData.map((item: any, index: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too!

Comment on lines 14 to 18
type DailyPoolStateProps = {
timestamp: string
tranches: { [trancheId: string]: TrancheState }
apy?: Perquintill | undefined
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather build the type from the existing dailyPoolState using Pick or Omit so that we can more easily catch changes if they were to happen

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for TrancheState as well

const data = React.useMemo(() => {
const tokenData =
dailyPoolStates?.map((state) => {
return { price: state.tranches[trancheId].price?.toFloat() || 0, day: new Date(state.timestamp) }
dailyPoolStates?.map((state: DailyPoolStateProps) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dailyPoolStates?.map((state: DailyPoolStateProps) => {
dailyPoolStates?.map((state) => {

again this type should be interfered if dailyPoolStates is type properly

import { Spinner } from '../Spinner'
import { Tooltips } from '../Tooltips'

const StyledPillButton = styled(PillButton)`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems this can be removed?

Comment on lines 33 to 45
interface DailyTrancheState {
yield30DaysAnnualized: Perquintill
timestamp: string
}

function hasValuationMethod(pricing: any): pricing is { valuationMethod: string } {
return pricing && typeof pricing.valuationMethod === 'string'
type DailyTrancheStateArr = Record<string, DailyTrancheState[]>

type Tranche = {
currency: {
name: string
}
id: string
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also here best to build the types from existing ones rather than redeclaring, recommend using Pick or Omit

const fmk = useFormikContext<PoolMetadataInput>()
const { values } = fmk

console.log('FORM', values)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

@kattylucy kattylucy requested review from sophialittlejohn and removed request for onnovisser and sophialittlejohn October 2, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants